feat: opt-out ssm parameters for github app #4335

AppliNH · 2025-01-06T09:45:39Z

Hi! 👋

Throughout my recent experiences, we had strong requirements to not have secret values in the state. To handle that, I suggest to make optional the creation of SSM parameters as part of the regular flow so it doesn't leak secrets in Terraform state.

We could alternatively use the latest ephemeral feature, but I'm not sure everyone is using Terraform 1.10+ atm since it's quite recent. I also think relying on ephemerals should be part of a new breaking release as it won't be compatible with Terraform < 1.10.

Let me know what you think of this!

…id having secrets in state

npalm

@AppliNH thx for the contribution.

Supporting external maanaged ssm parameters is fine. A view things to consider are

Update docs see docs dir to explain the usages
Explain or express via an example how to ajust the policies so the lambda can access the secrets. Lambda roles are exported.
I have added a comment related to the variables you introduces. I think it can done in a single app variable. Which avoid the introdduction of new variables. It also makes it easier to validat the variable.

variables.tf

examples/manual-ssm-parameters/README.md

…+ remove unused variables

AppliNH · 2025-02-13T14:42:49Z

Hi 👋 Really sorry for the delay, I've been terribly busy. I've updated everything, let me know what you think of it!

modules/ssm/variables.tf

variables.tf

npalm

I am busy with the review. I am fine with the approach. Please can you update the docs as well, see this page: https://github-aws-runners.github.io/terraform-aws-github-runner/configuration/ I think we should add this as major configuration choice. and / or make a remark in the ssm paramater section.

Please can you also make a note that the provided value has preference ofer the ssm paramater, at least this is the current order in reading the values via the terraform function coalense

I have tested th default example (with pool). Need to test the added example and the multi runners as well.

AppliNH · 2025-02-18T15:57:06Z

Please can you also make a note that the provided value has preference ofer the ssm paramater, at least this is the current order in reading the values via the terraform function coalense

@npalm
Reading the coalesce documentation, the SSM parameter has preference over the the value though.
Do you suggest it should be done otherwise?

Right now it's

github_app_parameters = {
    id             = coalesce(var.github_app.id_ssm, module.ssm.parameters.github_app_id)
    key_base64     = coalesce(var.github_app.key_base64_ssm, module.ssm.parameters.github_app_key_base64)
    webhook_secret = coalesce(var.github_app.webhook_secret_ssm, module.ssm.parameters.github_app_webhook_secret)
  }

npalm · 2025-02-18T16:11:57Z

Would keep the order in this way

docs/configuration.md

examples/external-managed-ssm-secrets/README.md

examples/external-managed-ssm-secrets/ssm.sh

examples/external-managed-ssm-secrets/main.tf

npalm · 2025-02-19T21:19:24Z

@AppliNH I think we are all most done. I have tested the relevant examples and the newly added example. Also made some changes directly to the PR. Let me know what you think.

examples/external-managed-ssm-secrets/README.md

AppliNH

I've reviewed and tested it all again, LGTM!
It's provisioned in production for us 👌

@AppliNH

🤖 I have created a release *beep* *boop* --- ## [6.3.0](v6.2.3...v6.3.0) (2025-02-24) ### Features * opt-out ssm parameters for github app ([#4335](#4335)) ([a2f013f](a2f013f)) @AppliNH ### Bug Fixes * **lambda:** bump the aws group in /lambdas with 6 updates ([#4440](#4440)) ([b10d87e](b10d87e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: runners-releaser[bot] <194412594+runners-releaser[bot]@users.noreply.github.com>

AppliNH and others added 3 commits January 6, 2025 10:40

feat(ssm): condition ssm parameters creation to a new variable to avo…

499c7a1

…id having secrets in state

docs(examples): add a new example to use manual ssm parameters option

ccfb39a

Merge branch 'main' into main

70c0a2a

npalm self-requested a review January 9, 2025 14:00

npalm requested changes Jan 9, 2025

View reviewed changes

variables.tf Outdated Show resolved Hide resolved

examples/manual-ssm-parameters/README.md Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main'

4c18eab

AppliNH requested review from a team as code owners February 13, 2025 14:00

github-actions bot and others added 5 commits February 13, 2025 14:00

docs: auto update terraform docs

65a0755

mv(example external-managed-ssm): change example name and refactor doc

fd65672

other(github_app): include ssm parameters inside existing github_app …

b613805

…+ remove unused variables

Merge branch 'main' of github.com:AppliNH/terraform-aws-github-runner

e313ecf

docs: auto update terraform docs

f5b6a5a

AppliNH requested a review from npalm February 13, 2025 14:42

AppliNH and others added 6 commits February 13, 2025 15:58

fix(modules ssm): fix outputs

c653599

Merge branch 'main' of github.com:AppliNH/terraform-aws-github-runner

a73d557

docs: auto update terraform docs

201a973

fix(ssm): fix module.ssm refs

c2d2cd1

fix(ssm ouputs): use simplier condition as coalesce cant be used here

556bca1

Merge branch 'main' of github.com:AppliNH/terraform-aws-github-runner

2ced074

npalm reviewed Feb 18, 2025

View reviewed changes

modules/ssm/variables.tf Show resolved Hide resolved

npalm reviewed Feb 18, 2025

View reviewed changes

variables.tf Outdated Show resolved Hide resolved

npalm reviewed Feb 18, 2025

View reviewed changes

AppliNH and others added 2 commits February 18, 2025 16:41

other(vars github_app): add validation block and update description

91723f0

docs: auto update terraform docs

9b3b65e

AppliNH and others added 2 commits February 18, 2025 17:03

other(vars github_app): add note regarding precedence

64bda92

docs: auto update terraform docs

af32f7e

doc: add note regarde github app secrets in SSM

3b799b2

AppliNH commented Feb 18, 2025

View reviewed changes

docs/configuration.md Show resolved Hide resolved

npalm and others added 2 commits February 19, 2025 21:52

chore: adjust examples and add script to easy test example

e96a560

docs: auto update terraform docs

328eb74

npalm reviewed Feb 19, 2025

View reviewed changes

docs/configuration.md Outdated Show resolved Hide resolved

npalm reviewed Feb 19, 2025

View reviewed changes

examples/external-managed-ssm-secrets/README.md Show resolved Hide resolved

add script to setup ssm

12bffc8

npalm reviewed Feb 19, 2025

View reviewed changes

examples/external-managed-ssm-secrets/ssm.sh Show resolved Hide resolved

npalm reviewed Feb 19, 2025

View reviewed changes

examples/external-managed-ssm-secrets/main.tf Show resolved Hide resolved

npalm and others added 5 commits February 20, 2025 09:21

Merge branch 'main' into main

e946698

doc: clean docs

89673da

docs: add example to index

4422e93

other(ssm-script docs): escape double quotes and refactor doc

8006a81

docs: auto update terraform docs

a1ef867

AppliNH commented Feb 20, 2025

View reviewed changes

examples/external-managed-ssm-secrets/README.md Show resolved Hide resolved

AppliNH requested a review from npalm February 20, 2025 15:18

npalm added 2 commits February 20, 2025 16:50

chore(ci): add new wexample to workflow

0c4a797

chore: small fixes

ad26106

AppliNH commented Feb 22, 2025

View reviewed changes

npalm approved these changes Feb 24, 2025

View reviewed changes

npalm changed the title ~~feat: make ssm parameters automatic creation optional to avoid having sensitive data in the state~~ feat: opt-out ssm parameters for github app Feb 24, 2025

npalm merged commit a2f013f into github-aws-runners:main Feb 24, 2025
42 checks passed

runners-releaser bot mentioned this pull request Feb 24, 2025

chore(main): release 6.3.0 #4435

Merged

npalm mentioned this pull request Mar 5, 2025

Provide existing SSM parameters for github_app secrets to avoid storing values in Terraform state #4420

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: opt-out ssm parameters for github app #4335

feat: opt-out ssm parameters for github app #4335

AppliNH commented Jan 6, 2025 •

edited

Loading

npalm left a comment

AppliNH commented Feb 13, 2025 •

edited

Loading

npalm left a comment

AppliNH commented Feb 18, 2025 •

edited

Loading

npalm commented Feb 18, 2025

npalm commented Feb 19, 2025

AppliNH left a comment •

edited

Loading

feat: opt-out ssm parameters for github app #4335

feat: opt-out ssm parameters for github app #4335

Conversation

AppliNH commented Jan 6, 2025 • edited Loading

npalm left a comment

Choose a reason for hiding this comment

AppliNH commented Feb 13, 2025 • edited Loading

npalm left a comment

Choose a reason for hiding this comment

AppliNH commented Feb 18, 2025 • edited Loading

npalm commented Feb 18, 2025

npalm commented Feb 19, 2025

AppliNH left a comment • edited Loading

Choose a reason for hiding this comment

AppliNH commented Jan 6, 2025 •

edited

Loading

AppliNH commented Feb 13, 2025 •

edited

Loading

AppliNH commented Feb 18, 2025 •

edited

Loading

AppliNH left a comment •

edited

Loading